FIX: Show local-axis grab measurement as a single tilted component#4326
FIX: Show local-axis grab measurement as a single tilted component#4326Chetansahney wants to merge 3 commits into
Conversation
When grabbing (G) and pressing X/Y a second time to constrain to a local axis, the measurement overlay drew an axis-aligned box showing both the X and Y screen components (and printed a typed value on both axes) instead of a single line along the tilted local axis. In the Grabbing overlay branch, when constrained to a single axis in local space, draw one dashed line along the (already tilted) translation vector with a single measurement label, mirroring how Scale (S) already behaves. The unconstrained and global-axis cases are unchanged. Closes GraphiteEditor#3341
There was a problem hiding this comment.
Code Review
This pull request improves the grab transform operation overlay. When translating a layer constrained to a single axis in local space, it now draws a single dashed line with a measurement label along that axis instead of an axis-aligned box. The reviewer suggested reusing the existing format_rounded helper to format the measurement label to maintain consistency and reduce code duplication.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Address review feedback: - Instead of hand-drawing the dashed line and label in the message handler, build the measurement quad in the layer's local frame and let the existing translation_box overlay code draw it. This removes the duplicated label formatting and the magic text-layout constants. - Use constraint_axis(..).is_some() as the branch condition rather than comparing against Axis::Both. - Convert the viewport translation to document space with document_to_viewport.inverse().transform_vector2() instead of dividing by the length of a matrix column. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@0HyperCube The main change: instead of hand-drawing the dashed line and label in the message handler, the measurement quad is now built in the layer's local (tilted) frame and handed to the existing translation_box. Because Quad holds four arbitrary corners and translation_box draws its X edge from top_left to top_right, this renders the single tilted line and one label with no new drawing code. The handler branch went from 13 lines to 3. That folds in the rest of the feedback as a side effect: Drawing now lives in translation_box rather than the handler. The magic constants 16 and 24 are gone, along with the duplicated label formatting, since translation_box already handles both including the typed-string case. This also covers the format_rounded suggestion from gemini-code-assist, as the manual formatting no longer exists. The branch condition is now constraint_axis(axis_constraint).is_some() && is_transforming_in_local_space, dropping the != Axis::Both comparison and the unwrap_or. I used .is_some() rather than binding the axis, since the vector itself is no longer needed once the quad is built in local space. The matrix-column division is gone. The label length now comes from document_to_viewport.inverse().transform_vector2(translation_viewport).length(), which is the proper viewport-to-document conversion and stays correct under a non-uniform viewport transform. Verified locally: cargo check -p graphite-editor passes, cargo clippy reports no warnings in the changed file, and cargo fmt --check is clean. |
Closes #3341
When grabbing (G) and pressing X/Y a second time to switch from a global-axis to a local-axis constraint, the on-canvas measurement overlay was drawn incorrectly. It showed an axis-aligned box with both the X and Y screen-space components, and when typing a value it printed that number on both axes, instead of a single line along the tilted local axis.
This implements the change requested in #3527 (comment).
Change
In the
Grabbingoverlay branch, when the transform is constrained to a single axis in local space, draw one dashed line from the pivot along the (already correctly tilted)translation_viewportvector, with a single measurement label — instead of the screen-alignedtranslation_box.The unconstrained and global-axis cases are untouched and still use
translation_box. The equivalent overlay for Scale (S) already behaved this way, so this brings Grab in line with it.